Skip to content

Conversation

@roomote
Copy link
Contributor

@roomote roomote bot commented Nov 16, 2025

This PR attempts to address the broader tool usage issues reported in #9239 and #9298. Feedback and guidance are welcome.

Problem

The removeClosingTag function was being incorrectly used in the execute methods of several tools. This function is designed to clean up partial XML tag artifacts during streaming (in handlePartial methods), but was being misused on actual parameter values like file paths.

This caused:

  • File paths containing or ending with "str" to be corrupted (e.g., a path like /root/oxyde_strat/stratoxyde-v2/src/services/storage/saveStates.ts would create a file named "str")
  • Tool execution failures across multiple models, not just x-ai/grok models
  • "Edit Unsuccessful" errors when models tried to use file tools

Root Cause

The removeClosingTag function uses a regex pattern to remove partial closing XML tags that might appear at the end of streamed content. When incorrectly applied to actual file paths in execute methods, it would strip characters that happen to match partial tag patterns, corrupting the paths.

Solution

  • Removed incorrect removeClosingTag calls from execute methods in:
    • WriteToFileTool
    • GenerateImageTool
    • accessMcpResourceTool (also fixed TypeScript type issue for partial messages)
  • These tools now use parameter values directly without modification
  • removeClosingTag remains available for its intended purpose in handlePartial methods

Changes

  • Fixed WriteToFileTool.ts to use paths directly
  • Fixed GenerateImageTool.ts to use paths directly
  • Fixed accessMcpResourceTool.ts to use values directly and handle undefined values in partial messages

Testing

  • All existing tool tests pass
  • Verified paths with "str" are handled correctly

Related Issues

Impact

This fix ensures that all tools correctly handle file paths and other parameters without corruption, improving reliability across all AI models using the tool system.


Important

Fixes file path corruption by removing incorrect removeClosingTag calls in tool execution methods.

This description was created by Ellipsis for b470bb0. You can customize this summary. It will automatically update as commits are pushed.

- Remove incorrect removeClosingTag calls from execute methods in tools
- removeClosingTag should only be used in handlePartial for streaming
- Fixes issue where paths ending with "str" were corrupted
- Fixed in WriteToFileTool, GenerateImageTool, and accessMcpResourceTool

This addresses the broader tool usage issue reported in #9239 and #9298
where incorrect usage of removeClosingTag was causing path corruption
and tool failures across multiple models.
@roomote roomote bot requested review from cte, jr and mrubens as code owners November 16, 2025 10:02
@dosubot dosubot bot added size:S This PR changes 10-29 lines, ignoring generated files. bug Something isn't working labels Nov 16, 2025
@roomote
Copy link
Contributor Author

roomote bot commented Nov 16, 2025

Rooviewer Clock   See task on Roo Cloud

Review complete. The changes correctly remove incorrect removeClosingTag usage from tool execution methods, fixing the file path corruption issue. All modified code looks good.

Mention @roomote in a comment to request specific changes to this pull request or fix all unresolved issues.

@bozoweed
Copy link

duplicat of #9299 you can close that

@hannesrudolph hannesrudolph added the Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. label Nov 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. size:S This PR changes 10-29 lines, ignoring generated files.

Projects

Status: Triage

Development

Successfully merging this pull request may close these issues.

[BUG] Unable to acces correctly to file in some path [BUG] x-ai/grok-code-fast-1 Edit Unsuccessful

4 participants